Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

t/porting/libperl.t: add better diagnostics #22447

Merged
merged 1 commit into from
Aug 2, 2024
Merged

t/porting/libperl.t: add better diagnostics #22447

merged 1 commit into from
Aug 2, 2024

Conversation

iabyn
Copy link
Contributor

@iabyn iabyn commented Jul 30, 2024

If it fails to find a symbol where expected (e.g. in the text section of the object file), then print a diagnostic message which lists all the nm lines where the symbol was found (if any).

This is achieved by adding a xref hash which maps symbol names to arrays of lines / object files.

Also document the format of the %symbols hash.

t/porting/libperl.t Outdated Show resolved Hide resolved
t/porting/libperl.t Outdated Show resolved Hide resolved
t/porting/libperl.t Outdated Show resolved Hide resolved
@iabyn
Copy link
Contributor Author

iabyn commented Jul 31, 2024

I've pushed an updated version. I've make it look for cross-ref symbol candidates by just grabbing any words on each line, rather than by just using parsed symbols on every known-format branch. So its more likely to find lines where the symbol appears, even if the line hasn't been well-parsed (which is kind of the idea behind these new diagnostics anyway).


# generate a cross-ref of every line each symbol appears on,
# for diagnostics
for my $sym (grep !/^[0-9A-Fa-f]+$/, /\b(\w{2,})\b/g) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[0-9A-Fa-f] could be written as [[:xdigit:]], which I like better.
$ is semantically wrong; should be \z.
But I'd probably invert the whole thing and write grep /[^[:xdigit:]]/, ... ("must contain a non-hex-digit").

(The second regex could be just /\w{2,}/g, but the explicit form is fine, too.)

for my $sym (grep !/^[0-9A-Fa-f]+$/, /\b(\w{2,})\b/g) {
push @{$symbols->{xref}{$sym}},
sprintf "%20s: %s", $symbols->{o}, $line;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate code. Maybe factor out into subroutine?

If it fails to find a symbol where expected (e.g. in the text section of
the object file), then print a diagnostic message which lists all the nm
lines where the symbol *was* found (if any).

This is achieved by adding a xref hash which maps symbol names to arrays
of lines / object files.

Also document the format of the %symbols hash.
@iabyn
Copy link
Contributor Author

iabyn commented Aug 1, 2024

Updated based on feedback and rebased. Third time lucky?

@jkeenan
Copy link
Contributor

jkeenan commented Aug 1, 2024

Updated based on feedback and rebased. Third time lucky?

In the commit message you say: "If it fails to find a symbol where expected (e.g. in the text section of the object file), then print a diagnostic message which lists all the nm lines where the symbol was found (if any)."

Is there any way I could trick the program into printing out that diagnostic?

@iabyn
Copy link
Contributor Author

iabyn commented Aug 1, 2024 via email

@jkeenan
Copy link
Contributor

jkeenan commented Aug 2, 2024

On Thu, Aug 01, 2024 at 08:07:19AM -0700, James E Keenan wrote: Is there any way I could trick the program into printing out that diagnostic?
Yes. For example, consider this chunk of code: has_symbol('PL_hash_seed_w', $symbols{data}{common}{PL_hash_seed_w}{'globals.o'}, "has PL_hash_seed_w"); To get the "but it was seen in these places" diagnostic, change the 'data' to 'XXX' in the code above so that it fails to find the symbol in the 'XXX' section. To get the "nor was it seen elsewhere" diagnostic, do s/PL_hash_seed_w/XXX/g in the lines above.

Confirmed. Thanks.

@iabyn iabyn merged commit f35bf03 into blead Aug 2, 2024
67 checks passed
@iabyn iabyn deleted the davem/libperl branch August 2, 2024 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants